Two fixes addressing issue 723#743
Two fixes addressing issue 723#743progirep wants to merge 1 commit intoNeuralNetworkVerification:masterfrom
Conversation
- Using a non-two-D Convolution Layer now gives an error message - If the layer sizes are stored in INT64 instead of INT32, this now works
MatthewDaggitt
left a comment
MatthewDaggitt
left a comment
There was a problem hiding this comment.
Thanks for the PR! Generally looks great, but a few comments.
| TensorShape inputShape = _shapeMap[inputNodeName]; | ||
|
|
||
| // Added: Check if convolutional layers are only 2D | ||
| if (inputShape.size()!=4) { |
There was a problem hiding this comment.
Very minor but this should be laid out as:
if ( inputShape.size() != 4 )
{
and tabs should be four spaces. Can you check that this style is used everywhere?
| result.append( value ); | ||
| } | ||
| } else { | ||
| String errorMessage = Stringf( "Illegal data type for integer tensors used in the model. Only INT32 and INT64 supported." ); |
There was a problem hiding this comment.
This should be "unsupported" not "illegal"?
| } else { | ||
| String errorMessage = Stringf( "Illegal data type for integer tensors used in the model. Only INT32 and INT64 supported." ); | ||
| throw MarabouError( MarabouError::ONNX_PARSER_ERROR, errorMessage.ascii() ); | ||
| } |
There was a problem hiding this comment.
I think you also need to add a corresponding case for the else below when the data is not stored in raw_data. I'd advice lifting the dataType check above the raw_data check.
|
@progirep Thank you so much for your code contribution. I have two requests (sorry for the trouble)!
|
3 participants
This is supposed to address issue 723: #723
There are two changes:
The current code is not clean in that probably the error message formatting is not exactly how this is envisioned in the project, the code formatting is not great, and data types are not checked uniformly. The additional checks for data types probably make the overall code a tiny bit slower, but not much.